Fix: added modal for expired Domain #1075
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@viktrified is attempting to deploy a commit to the LFG Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe update modifies the navbar component by introducing a new state variable for controlling a modal's visibility related to domain expiration. When users click the "Domains" menu item, the navbar sets the state to display a modal message. The modal shows a title, a message about renewing a domain, and features a button that triggers an alert to indicate the renewal process has started. The component then resets the modal’s state when it is closed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant N as Navbar Component
participant M as ModalMessage Component
U->>N: Click "Domains" menu item
N->>N: Set showModalExpiredDomain = true
N-->>M: Render modal ("Domain Expiration Notice")
U->>M: Click "Renew" button
M->>U: Display alert("Renewal process initiated")
U->>N: Close modal
N->>N: Set showModalExpiredDomain = false
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/UI/navbar.tsx (1)
454-454: Implement or remove commented lottie animation.The commented lottie animation prop suggests incomplete implementation.
Either implement the animation for better UX or remove the commented code:
-// lottie={errorLottie} +lottie={domainExpirationLottie}Note: You'll need to import the appropriate lottie animation file for domain expiration.
| your website active? | ||
| </p> | ||
| <div className="mt-5"> | ||
| <Button onClick={() => alert('Renew Domain clicked')}>Renew Domain</Button> |
There was a problem hiding this comment.
Replace alert with proper domain renewal handling.
Using alert() for domain renewal is not production-ready and provides poor user experience.
Consider implementing a proper domain renewal handler:
-<Button onClick={() => alert('Renew Domain clicked')}>Renew Domain</Button>
+<Button onClick={() => handleDomainRenewal()}>Renew Domain</Button>Add the following handler function to manage the renewal process:
const handleDomainRenewal = async () => {
try {
// Implement domain renewal logic here
setShowModalExpiredDomain(false);
} catch (error) {
// Handle errors appropriately
console.error('Domain renewal failed:', error);
}
};| <li | ||
| className={styles.menuItem} | ||
| onClick={() => setShowModalExpiredDomain(true)} | ||
| > | ||
| Domains | ||
| </li> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve UX by separating navigation and modal trigger.
The current implementation is potentially confusing as it uses a Link component but prevents navigation with onClick. This could lead to unexpected behavior and poor user experience.
Consider one of these approaches:
- Remove the Link wrapper if navigation is not intended:
-<Link href="/">
<li
className={styles.menuItem}
onClick={() => setShowModalExpiredDomain(true)}
>
Domains
</li>
-</Link>- Or handle both navigation and modal display if both are needed:
-<Link href="/">
+<Link href="/domains">
<li
className={styles.menuItem}
onClick={(e) => {
+ e.preventDefault();
setShowModalExpiredDomain(true);
}}
>
Domains
</li>
</Link>Committable suggestion skipped: line range outside the PR's diff.
|
@viktrified please add close: #issue-id in your PR comment |
|
Close #1024 |
|
@viktrified are you on it ? |
close: #1024
Summary by CodeRabbit